feat: filter sub-language read half — runtime + studio (RFD 0021)#4
Conversation
Implements the read half of the filter sub-language: one owned predicate IR, two borrowed frontends, one parameterized SQLite WHERE. Discharges the pagination/ordering debt for derived read surfaces. Runtime (crates/spock-runtime): - src/filter.rs (new): the predicate IR (And/Or/Not/Cmp/In/IsNull/Const) and injection-proof lowering — operators from a closed enum, columns validated + double-quoted, every value a bound `?`. Owns the forced stable total order (user terms + pk tiebreak in the last direction, explicit NULLS placement) and paging (page cap + offset-depth ceiling). Includes the PostgREST frontend (parse_rest) with recursive and/or/not groups and not.-prefix. - graphql.rs: the Hasura bool_exp frontend — per-table <t>_bool_exp / <t>_order_by inputs + <scalar>_comparison_exp + the order_by enum; where / order_by / offset args on list roots and reverse collections; query_rows rewritten onto the one composer. - http.rs: list_rows takes ordered query params and lowers via parse_rest; reserved-column startup guard (order/limit/offset/select/and/or/not). - v0 ops: eq/neq/gt/gte/lt/lte/in/nin/is_null/ilike + and/or/not. `_like` (case-sensitive) is refused — the SQLite floor is ASCII-case-insensitive and the case-sensitive form needs the banned PRAGMA. Postgres-only tail is a loud bad_request. Ref fields typed as <target>_bool_exp; the key sub-field folds to a direct FK compare. Keyset cursors deferred (offset + 10k ceiling). Studio (crates/spock-runtime/studio) — the Supabase-style console surface: - views/table-view.tsx: Filter + Sort popovers (Supabase table-editor UX) and an offset pager, wired to the REST predicate. Operator menu is symbol+label; closed-set/bool columns get value dropdowns; unary is-null/not-null hide the value. Refusals surface in the grid. Re-fetches only when the effective query changes. - components/ui/popover.tsx (new): shadcn wrapper over @base-ui/react/popover. - lib/query.ts (new): the operator model + the filter/sort/page -> PostgREST query-string builder (the client mirror of filter.rs). Fixture + docs: - examples/filter-lab (new): a domain-less technical fixture (widget + edge) stressing every scalar/closed-set/nullable/ref/composite-key path plus ordering ties, NULL placement, %/_ wildcard escaping and ASCII case folding; FEEDBACK.md records the F1-F7 dogfood findings. - docs: RFD 0021 -> accepted/read-half shipped; RFD 0009 roadmap marks the filter dialect ratified/shipped; graphql.md §7 strikes `_like`. Verified: 201 tests + clippy + fmt green; studio tsc/oxlint/pnpm build clean; read path exercised end-to-end (REST + GraphQL + studio) against filter-lab. Filtered/bulk writes and the v1 policy engine build on this same IR (deferred).
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughChangesThe runtime now provides a shared predicate IR for REST and GraphQL filtering, ordering, pagination, validation, and bound SQLite queries. The studio adds filter and sort controls, while integration tests, a filter fixture, and RFD/spec documents describe and validate the read surface. Filter read surface
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RESTOrGraphQL
participant PredicateParser
participant query_rows
participant SQLite
Client->>RESTOrGraphQL: filter, order, limit, offset
RESTOrGraphQL->>PredicateParser: parse request
PredicateParser->>query_rows: Predicate and ordering
query_rows->>SQLite: bounded SQL with parameters
SQLite-->>Client: ordered result page
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/spock-runtime/studio/src/views/table-view.tsx (1)
138-156: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStale response can overwrite fresher rows — no request sequencing.
load()has no guard against out-of-order responses. Filter/sort/paging edits each independently callreloadWith/maybeLoad→load(), and with several rapid edits in flight, an older request's response can resolve after a newer one and silently replace the up-to-date rows/error state with stale data.
this.lastQueryis already updated synchronously right before theawait, so it can double as a staleness check on the way back:🩹 Proposed fix
const qs = buildQuery(filters, sorts, limit, offset) this.lastQuery = qs const res = await api(`/rest/v1/${encodeURIComponent(table.name)}?${qs}`, this.context.actor) + // a newer load() may have started (and rewritten lastQuery) while this + // request was in flight — drop the now-stale response + if (qs !== this.lastQuery) return if (res.status !== 200) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/spock-runtime/studio/src/views/table-view.tsx` around lines 138 - 156, Update the async load method to ignore stale responses: after the await api call and before applying either the error state or rows, compare the request’s captured query against this.lastQuery and return when they differ. Preserve the existing state updates for the latest request only, using the synchronous lastQuery assignment in load as the request-sequencing marker.
🧹 Nitpick comments (2)
crates/spock-runtime/src/filter.rs (1)
502-514: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
ilikeisn't restricted to text columns on the REST frontend.Unlike the GraphQL side (where
_ilikeonly exists onString_comparison_exp),parse_column_opbuildsCmpOp::Ilikefor any resolved column without checking its value type, so?age=ilike.5*on an integer column silently produces aLIKEon non-text data (SQLite coerces rather than erroring). This is a safe-degradation gap, but it diverges from the module's fail-loud stance elsewhere (set membership,is.true/falseboolean check). Consider rejectingilikeon non-text/set columns with atype_mismatch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/spock-runtime/src/filter.rs` around lines 502 - 514, Update parse_column_op’s "ilike" branch to validate that the resolved column is text-compatible before constructing Predicate::Cmp. Reject non-text and set-valued columns with the existing type_mismatch error, while preserving wildcard replacement and CmpOp::Ilike behavior for text columns.crates/spock-runtime/tests/graphql.rs (1)
941-959: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComment promises
_and, test doesn't exercise it.The header comment lists
_and/_or/_not, but only_or(line 944) and_not(line 952) are tested. No case buildswhere: {_and: [...]}.Suggested addition
let resp = gql( &base, r#"{ user(where: {_not: {username: {_eq: "maya"}}}) { username } }"#, Value::Null, ) .await; assert_no_errors(&resp); let users = resp["data"]["user"].as_array().unwrap(); assert_eq!(users.len(), 1); assert_eq!(users[0]["username"], "luis"); + + let resp = gql( + &base, + r#"{ post(where: {_and: [{caption: {_ilike: "%first%"}}, {caption: {_ilike: "%light%"}}]}) { caption } }"#, + Value::Null, + ) + .await; + assert_no_errors(&resp); + assert_eq!(resp["data"]["post"].as_array().unwrap().len(), 1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/spock-runtime/tests/graphql.rs` around lines 941 - 959, Add an explicit GraphQL integration case in the `_and / _or / _not` test block that queries `where: {_and: [...]}` with multiple caption predicates, then assert no errors and verify the returned rows match the expected intersection. Keep the existing `_or` and `_not` assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/spock-runtime/src/filter.rs`:
- Around line 417-465: Add a maximum nesting depth of 32 to the recursive
parse_group/parse_group_member flow, passing and incrementing the depth as
nested groups are parsed and returning ApiError::bad_request when the limit is
exceeded. Ensure both normal and negated and/or groups enforce the cap before
recursing, preventing unbounded stack growth and substring rebuilding.
In `@crates/spock-runtime/src/graphql.rs`:
- Around line 873-905: The parse_order_json function must not silently reorder
keys in the lone-object order_by form. Reject object inputs containing multiple
fields, or otherwise restrict multi-key ordering to the array/list form, while
preserving existing validation and support for single-key objects.
In `@crates/spock-runtime/studio/src/views/table-view.tsx`:
- Around line 186-193: Update fSetColumn so changing a filter column also resets
its existing value and operator to the appropriate empty/default state before
the reload. Preserve mapRule’s behavior and ensure the updated filter cannot
retain a value or op incompatible with the newly selected column.
---
Outside diff comments:
In `@crates/spock-runtime/studio/src/views/table-view.tsx`:
- Around line 138-156: Update the async load method to ignore stale responses:
after the await api call and before applying either the error state or rows,
compare the request’s captured query against this.lastQuery and return when they
differ. Preserve the existing state updates for the latest request only, using
the synchronous lastQuery assignment in load as the request-sequencing marker.
---
Nitpick comments:
In `@crates/spock-runtime/src/filter.rs`:
- Around line 502-514: Update parse_column_op’s "ilike" branch to validate that
the resolved column is text-compatible before constructing Predicate::Cmp.
Reject non-text and set-valued columns with the existing type_mismatch error,
while preserving wildcard replacement and CmpOp::Ilike behavior for text
columns.
In `@crates/spock-runtime/tests/graphql.rs`:
- Around line 941-959: Add an explicit GraphQL integration case in the `_and /
_or / _not` test block that queries `where: {_and: [...]}` with multiple caption
predicates, then assert no errors and verify the returned rows match the
expected intersection. Keep the existing `_or` and `_not` assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11b49102-4220-4284-9045-796e46b03d78
📒 Files selected for processing (15)
crates/spock-runtime/src/filter.rscrates/spock-runtime/src/graphql.rscrates/spock-runtime/src/http.rscrates/spock-runtime/src/lib.rscrates/spock-runtime/studio/src/components/ui/popover.tsxcrates/spock-runtime/studio/src/lib/query.tscrates/spock-runtime/studio/src/views/table-view.tsxcrates/spock-runtime/tests/filter.rscrates/spock-runtime/tests/graphql.rscrates/spock-runtime/tests/http.rsdocs/rfd/0009-roadmap.mddocs/rfd/0021-filter.mddocs/spec/graphql.mdexamples/filter-lab/FEEDBACK.mdexamples/filter-lab/schema.spock
- filter.rs: cap REST logical-group nesting at 32 (MAX_FILTER_DEPTH) — an
unbounded ?and=(and(and(...))) could overflow the stack / burn superlinear
work before returning bad_request. Mirrors GraphQL's .limit_depth(32).
- filter.rs: reject `ilike` on non-text columns (type_mismatch) — mirrors the
GraphQL side where `_ilike` lives only on String; SQLite would coerce silently.
- graphql.rs: reject multi-key `order_by` objects — serde_json::Map iterates in
sorted key order, so {b: desc, a: asc} silently reordered to a,b; the list
form carries multiple terms.
- studio/table-view.tsx: clear a filter's value when its column changes (a
wrong-typed value would type_mismatch and show a non-option in set/bool
dropdowns); drop stale in-flight load() responses via the lastQuery marker so
a slow older request can't clobber fresher rows.
- tests: add _and GraphQL case; add REST refusals for ilike-on-int, deep
nesting, and multi-key order_by.
All green: 201 tests + clippy + fmt; studio tsc/oxlint/pnpm build; column-clear
verified live.
1 participant
Summary
Implements the read half of the filter sub-language (RFD 0021), end to end — one owned predicate IR, two borrowed frontends (Hasura
bool_expfor GraphQL, PostgREST operators for REST), one parameterized SQLiteWHERE— and wires it into the studio console with a Supabase-style Filter/Sort UX. This was the roadmap's five-times-deferred query layer; it also discharges the pagination/ordering debt for derived read surfaces.Runtime (
crates/spock-runtime)src/filter.rs(new) — the predicate IR (And/Or/Not/Cmp/In/IsNull/Const) and injection-proof lowering: operators come from a closed enum through a fixed match arm, columns are validated + double-quoted, and every value is a bound?(the only length-variable emission is the?count in anINlist). Owns the forced stable total order (user terms + primary-key tiebreak in the last direction, with explicitNULLSplacement since SQLite's implicit default is inverted) and paging (page cap + offset-depth ceiling). Also houses the PostgREST frontendparse_rest(recursiveand/or/notgroups,not.prefix,is.{null,true,false}).graphql.rs— the Hasurabool_expfrontend: per-table<t>_bool_exp/<t>_order_byinputs,<scalar>_comparison_exp, theorder_byenum;where/order_by/offsetargs on list roots and reverse collections;query_rowsrewritten onto the single composer.http.rs—list_rowstakes ordered query params and lowers viaparse_rest; a startup guard reserves the control keys (order/limit/offset/select/and/or/not) against column names.v0 operators:
eq/neq/gt/gte/lt/lte/in/nin/is_null/ilike+and/or/not._like(case-sensitive) is refused — the SQLite floor'sLIKEis ASCII-case-insensitive and the case-sensitive form needs the bannedPRAGMA case_sensitive_like;_ilikeships. The Postgres-only tail (regex/fts/jsonb/…) is a loudbad_request.<target>_bool_exp; the key sub-field folds to a direct FK comparison (non-key sub-fields reserved →bad_request).Studio (
crates/spock-runtime/studio)views/table-view.tsx— replaces the "waits on the filter RFD" placeholder with a Filter popover ([column][operator][value]rows + active-count badge) and a Sort popover (column picker + asc/desc toggle), plus an offset pager, all wired to the REST predicate. The operator menu is symbol+label (likedeliberately absent, mirroring the floor's refusal); closed-set and bool columns get value dropdowns; unaryis null/is not nullhide the value; refusals (unknown_field/type_mismatch) surface in the grid. Re-fetches only when the effective query actually changes.components/ui/popover.tsx(new) — shadcn wrapper over@base-ui/react/popover, matching the existingselect.tsxwrapper.lib/query.ts(new) — the operator model + thefilter/sort/page → PostgRESTquery-string builder (the client mirror offilter.rs).Fixture + docs
examples/filter-lab/(new) — a deliberately domain-less technical fixture (widget+edge) that stresses every scalar / closed-set / nullable / ref / composite-key path plus the sharp edges: ordering ties, NULL placement,%/_wildcard escaping, ASCII case folding.FEEDBACK.mdrecords the F1–F7 dogfood findings.graphql.md§7 strikes_like.Verification
cargo test201 passed / 0 failed,cargo clippyclean,cargo fmt --checkclean.tsc+oxlint+pnpm build(production) clean.filter-labover REST, GraphQL, and the studio UI (filter, sort, enum-value dropdown,is not null, tie-stable ordering, the offset pager).Reviewer notes
spock-runtime(protocol), not thespock-langcontract IR — filters are per-request, never.spocksource. Both floors converge on one composer, which is the intended chokepoint for the v1policy/RLS dry-run (leaf-parametricOperand, reserved un-emittedExists)./simplifycleanup pass (helper extraction, dead-code removal, a plain-button sort picker replacing a misused controlledSelect)..claude/preview config and the gitignoredstudio/dist/bundle are intentionally not committed (CI rebuildsdist).